Repository navigation
Fix browser webview divider resizing - #7038
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCentralizes split-divider hit geometry, updates portal cursor and drag detection to use shared hit rectangles, and adds browser panel hit-test pass-through so nearby external split dividers can receive resize events. ChangesSplit divider hit testing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (23 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 6684-6724: `externalSplitDividerHit` currently duplicates the
divider geometry used by `PortalSplitDividerRegion.appendDividerRegions`, and
the hit expansion is split across `resetCursorRects`, `dividerCursorKind`, and
this helper. Update the hit-test path to derive the divider region from
`PortalSplitDividerRegion` (or shared geometry logic) and centralize the
expansion in one constant so `externalSplitDividerHit`, `resetCursorRects`, and
`dividerCursorKind` all use the same values and stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: df43144d-2310-4106-846b-e4948b9934af
📒 Files selected for processing (1)
Sources/Panels/BrowserPanelView.swift
Greptile SummaryThis PR fixes browser pane divider resizing by overriding
Confidence Score: 5/5Safe to merge. The hit-test pass-through is tightly guarded by a near-edge pre-filter and an explicit window-coordinate divider-rect confirmation, preserving normal webview clicks and scrolling. All changed paths are AppKit pointer-routing helpers on the main actor with no actor-isolation or concurrency concerns. The shared geometry helpers in PortalSplitDividerRegion are straightforward NSRect math with correct bounds-clamping. The dock-slot flag is scoped to the slot view and set at the same call site as its backing drop context. The cursor and hit-test pass-through logic are symmetric. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "Use Dock browser edge for sidebar resize..." | Re-trigger Greptile |
| private func isNearPaneEdge(_ point: NSPoint) -> Bool { | ||
| let expansion = PortalSplitDividerRegion.dividerHitExpansion | ||
| let nearVerticalEdge = point.x >= bounds.minX && | ||
| point.x <= bounds.maxX && | ||
| (point.x <= bounds.minX + expansion || point.x >= bounds.maxX - expansion) | ||
| let nearHorizontalEdge = point.y >= bounds.minY && | ||
| point.y <= bounds.maxY && | ||
| (point.y <= bounds.minY + expansion || point.y >= bounds.maxY - expansion) | ||
| return nearVerticalEdge || nearHorizontalEdge | ||
| } |
There was a problem hiding this comment.
isNearPaneEdge compares hitTest point in superview coordinates against bounds in local coordinates
NSView.hitTest(_:) receives point in the superview's coordinate system, but bounds.minX/bounds.maxX/bounds.minY/bounds.maxY are in the view's own coordinate system. The check works today because HostContainerView is always pinned to origin (0,0) inside its container (so superview coordinates coincide with local coordinates), but the assumption is invisible. If the view is ever laid out with a non-zero frame origin the near-edge predicate would silently misfire, causing either spurious pass-throughs or missed ones.
Consider converting the point to local coordinates first (let local = superview?.convert(point, to: self) ?? point) and comparing against bounds, or adding a comment that documents the origin-is-zero invariant.
There was a problem hiding this comment.
Addressed in f87f8e6: now documents and guards the local-bounds coordinate invariant, and mirrors the external split-divider pass-through path with so the underlying split cursor can take over.
— Claude Code
There was a problem hiding this comment.
Correction to the previous reply: addressed in f87f8e6. isNearPaneEdge now documents and guards the local-bounds coordinate invariant, and updateDividerCursor mirrors the external split-divider pass-through path with restoreArrow false so the underlying split cursor can take over.
— Claude Code
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/BrowserWindowPortal.swift`:
- Around line 821-822: In BrowserWindowPortal’s divider-hit logic, the current
contentSlots/maxX path can miss the Dock divider when the Dock is the only
browser pane or when the adjacent area is a terminal. Update the divider
coordinate lookup to prefer the marked dock slot’s frame.minX as the primary
source, then fall back to the existing non-dock content edge computation. Make
sure the dock-divider hit test in the same routing path fails closed when
neither source is available, rather than letting the WebView consume the
mouse-down.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c5e58930-ec89-4911-a727-8f94f344b924
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (1)
Sources/BrowserWindowPortal.swift
Summary
Fixes #7037
Browser panes now decline hits over real adjacent split-divider grab bands before descending into the hosted
WKWebView. This prevents WebKit's internal scrollbar view from winning the mouse-down when the pointer is in the pane divider band, while preserving normal webview clicks, scrolling, selection, and DevTools inspector divider hits outside that band.I used the
HostContainerView.hitTestpass-through alternative instead of adding a child overlay inpinHostedWebView(): a child overlay would become the hit view and then has to rely on responder-chain forwarding to reach the split view, while returningnilfrom the host over a verified external divider band routes directly through the existing bonsplit/SwiftUI resize paths. The webview frame stays unchanged.Verification
git diff --checkTest note
I did not add a regression test because the failure depends on
WKWebView's private internal scrollbar view winning AppKit hit-testing while layered over anNSSplitViewdivider. A unit test of the helper math would not prove the user-visible regression, and a meaningful test would need an end-to-end UI/AppKit/WebKit hit-test harness.Localization
No user-facing strings were added or changed. The only added string literal is a DEBUG hit-test log stage.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes divider resizing and cursor/hover behavior in browser panes (including the docked right sidebar) by centralizing divider hit geometry and passing near-edge divider hits through
WKWebViewto the parentNSSplitView. Also updates Swift file length budgets.PortalSplitDividerRegion(dividerHitExpansion,hitRectInWindow,dividerHitRect{,InWindow}) used across browser, terminal, and panel hit-testing.hitRectInWindowin both portals.Written for commit c3ea0b4. Summary will update on new commits.
Summary by CodeRabbit